Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid derivation of fixed fractionDigits #34973

Merged
merged 6 commits into from
Apr 29, 2020
Merged

Invalid derivation of fixed fractionDigits #34973

merged 6 commits into from
Apr 29, 2020

Conversation

mrj001
Copy link
Contributor

@mrj001 mrj001 commented Apr 14, 2020

@ghost
Copy link

ghost commented Apr 14, 2020

Tagging subscribers to this area: @buyaa-n
Notify danmosemsft if you want to be subscribed.

Copy link
Contributor

@buyaa-n buyaa-n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrj001 Thank you for noticing this and fixing, left one NIT comment, other than that looks good to me

@buyaa-n buyaa-n requested a review from krwq April 18, 2020 01:03
@mrj001
Copy link
Contributor Author

mrj001 commented Apr 20, 2020

@buyaa-n
I've changed the order of these two tests.

Note that I also had to removed "fixed='true'" from another test (FractionDigitsMismatch_Throws). It wasn't needed for this test, but at the time the test was created, it was not honored. Therefore, I hadn't noticed the error. With the fix to this issue, that test started failing due to a different exception being thrown now that the fixed facet was honored.

@buyaa-n
Copy link
Contributor

buyaa-n commented Apr 26, 2020

I've changed the order of these two tests.

Note that I also had to removed "fixed='true'" from another test (FractionDigitsMismatch_Throws). It wasn't needed for this test, but at the time the test was created, it was not honored. Therefore, I hadn't noticed the error. With the fix to this issue, that test started failing due to a different exception being thrown now that the fixed facet was honored.

Thank you @mrj001, the tests might fail on full framework too, might need to add SkipOnTargetFramework

mrj001 added 6 commits April 27, 2020 11:26
…o alter the fixed value of fractionDigits from the base type.
…Throws test as the wrong exception is being thrown by .NET Framework 4.7.2.

-removed extra blank line at end of file.
@mrj001
Copy link
Contributor Author

mrj001 commented Apr 27, 2020

@buyaa-n
I've added the SkipOnTargetFramework attribute to FractionDigitsFacetBaseFixed_Throws, as .NET Framework 4.7.2 did not throw the required exception.

.NET Framework 4.7.2 is also throwing the "totalDigits" error message in the FractionDigitsMismatch_Throws test instead of the "fractionDigits" error message. This would cause that test to fail under 47.2 as well. I've taken the liberty of adding the SkipOnTargetFramework attribute here as well, even though it is outside the scope of this pull request.

@buyaa-n
Copy link
Contributor

buyaa-n commented Apr 29, 2020

I've added the SkipOnTargetFramework attribute to FractionDigitsFacetBaseFixed_Throws, as .NET Framework 4.7.2 did not throw the required exception.

.NET Framework 4.7.2 is also throwing the "totalDigits" error message in the FractionDigitsMismatch_Throws test instead of the "fractionDigits" error message. This would cause that test to fail under 47.2 as well. I've taken the liberty of adding the SkipOnTargetFramework attribute here as well, even though it is outside the scope of this pull request.

Great, looks good, thank you @mrj001.

The CI legs showing still in progress were successfully finished in details on Azure Pipelines, gonna merge this PR

@buyaa-n buyaa-n merged commit 3735efb into dotnet:master Apr 29, 2020
@mrj001 mrj001 deleted the FractionDigitsIssue34418 branch April 30, 2020 15:58
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid derivation of fixed fractionDigits facet fails to throw XmlSchemaException
3 participants